Skip to content

Add basic FormSteps flow for IdV React implementation#6195

Merged
aduth merged 12 commits intomainfrom
aduth-personal-key-react
Apr 14, 2022
Merged

Add basic FormSteps flow for IdV React implementation#6195
aduth merged 12 commits intomainfrom
aduth-personal-key-react

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Apr 13, 2022

Why: So that we can start implementing step screens for the new, React-based identity verification flow.

Screenshot:

(Note that this is currently only implementing simple content, not intending to be a complete implementation)

image

@aduth
Copy link
Copy Markdown
Contributor Author

aduth commented Apr 13, 2022

Initial commits up being fewer changes than I anticipated, maybe I shall go ahead and build out more here!

@aduth aduth marked this pull request as draft April 13, 2022 13:42
@aduth aduth marked this pull request as ready for review April 13, 2022 13:56
@aduth aduth requested review from nprimak, peggles2 and solipet April 13, 2022 15:21
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment thread app/javascript/packages/verify-flow/steps/personal-key/personal-key-step.tsx Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason we can remove this here is because we switched to globalThis._locale_data right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the reason we can remove this here is because we switched to globalThis._locale_data right?

Yeah, indirectly that's what the previous logic here was doing anyways, since the default i18n instance from @18f/identity-i18n is also initialized with globalThis._locale_data.

const i18n = new I18n({ strings: globalThis._locale_data });

Now that I'm thinking about it, I wonder if there's a better way for the React context to reference that default i18n, rather than creating a new instance 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I'm thinking about it, I wonder if there's a better way for the React context to reference that default i18n, rather than creating a new instance 🤔

Updated in 0997d97. The value of an I18nContext is pretty low at this point since we have the shared instance to use now. Technically it could allow to use different locale data specific contexts, though I can't imagine we'd ever want to do that.

Might consider a future refactor:

import { t } from '@18f/identity-i18n';
import { formatHTML } from '@18f/identity-react-i18n';

Or maybe even merge the packages.

import { t, formatHTML } from '@18f/identity-i18n';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically it could allow to use different locale data specific contexts, though I can't imagine we'd ever want to do that.

I guess there's one use-case after all 😅 77952d5b6

@aduth aduth force-pushed the aduth-verify-api-route branch from fc7fb1b to 446e4b5 Compare April 13, 2022 16:22
@aduth aduth force-pushed the aduth-personal-key-react branch from 77952d5 to 57a439d Compare April 13, 2022 16:23
Base automatically changed from aduth-verify-api-route to main April 14, 2022 12:21
aduth added 12 commits April 14, 2022 08:22
For reuse in verify-flow
**Why**: So that we can start implementing step screens for the new, React-based identity verification flow.

changelog: Upcoming Features, Identity Verification, Add personal key step screen
Moving toward potential future removal of useI18n
Previously receives strings object, now receives I18n instance
Bit easier to read with className last
@aduth aduth force-pushed the aduth-personal-key-react branch from 34276b8 to 85de52f Compare April 14, 2022 12:24
@aduth aduth merged commit 7b0a171 into main Apr 14, 2022
@aduth aduth deleted the aduth-personal-key-react branch April 14, 2022 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants